Sanitize gateway token #532
Conversation
|
Caution Review failedThe pull request is closed. Walkthroughcreate_pathspec_from_gitignore now returns None when the ignore file is missing and should_ignore_by_gitignore short-circuits when spec is None; workflow debug logging masks gateway_token (pattern-aware masking); tests were reformatted or reflowed without behavioral changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DeployCLI as Deploy CLI
participant BundleUtils as Bundle Utils
note over DeployCLI,BundleUtils: Packaging with gitignore handling
User->>DeployCLI: run deploy
DeployCLI->>BundleUtils: create_pathspec_from_gitignore(path)
alt .gitignore exists
BundleUtils-->>DeployCLI: PathSpec
else .gitignore missing
BundleUtils-->>DeployCLI: None
end
DeployCLI->>BundleUtils: should_ignore_by_gitignore(path_str, names, project_dir, spec)
alt spec is None
BundleUtils-->>DeployCLI: return empty ignored set (short-circuit)
else spec provided
BundleUtils-->>DeployCLI: return ignored names set
end
sequenceDiagram
autonumber
participant Executor as Executor Workflow
participant Logger as Debug Logger
note over Executor,Logger: Mask gateway_token before logging
Executor->>Executor: build memo_map (dict)
Executor->>Executor: sanitize gateway_token (pattern-aware)
Executor->>Logger: log debug with sanitized token
Logger-->>Executor: debug output (masked)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
src/mcp_agent/cli/cloud/commands/deploy/bundle_utils.py(1 hunks)src/mcp_agent/executor/workflow.py(1 hunks)tests/cli/commands/test_deploy_command.py(1 hunks)tests/cli/commands/test_wrangler_wrapper.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/mcp_agent/executor/workflow.py (1)
src/mcp_agent/logging/logger.py (1)
debug(261-269)
tests/cli/commands/test_wrangler_wrapper.py (2)
src/mcp_agent/cli/cloud/commands/deploy/bundle_utils.py (1)
should_ignore_by_gitignore(41-80)src/mcp_agent/cli/cloud/commands/deploy/wrangler_wrapper.py (1)
wrangler_deploy(114-394)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks / test
🔇 Additional comments (5)
tests/cli/commands/test_deploy_command.py (1)
120-135: LGTM! Test structure improved.Moving the
runner.invokecall inside thewithpatch context improves readability without changing behavior—the mocks remain active during the invocation either way.tests/cli/commands/test_wrangler_wrapper.py (2)
1040-1040: LGTM! Formatting consistency.Quote style updated for consistency with the project's conventions.
1203-1205: LGTM! Improved readability.Multi-line formatting makes the function call easier to read without changing behavior.
src/mcp_agent/cli/cloud/commands/deploy/bundle_utils.py (2)
17-38: LGTM! Robust handling of missing files.The existence check and explicit
Nonereturn allow callers to gracefully handle missing ignore files and fall back to default behavior. The updated docstring clearly documents this contract.
56-57: LGTM! Efficient short-circuit.The early return when
spec is Noneavoids unnecessary path iteration and clearly expresses the intent that no-spec means no-ignores.
|
@rholinshead Too late to comment, but I just want to say I always find it super helpful when the first (or last) few distinguishing characters are revealed. It not only helps confirming that a token is set, but also helps to confirm that it's the right token |
3 participants
Summary
Realized, while looking through my app logs, that the raw gateway token is output in the logs. We need to sanitize it. Just doing first 6 chars for now (which will effectively be
lm_mcp...) to make it clear that a token is set. Not sure if we should increase the length just in case some other token ends up being used with a smaller length.Tests
Summary by CodeRabbit